Skip to content

Conversation

@gbeane
Copy link
Collaborator

@gbeane gbeane commented Jan 15, 2026

Fix "training report close button disabled on windows"

it turns out this problem, which was specific to Windows, was related to some code that temporarily changed window flags and forced the dialog to redisplay in an attempt to ensure it popped up as the active window when training finished if it was already open and hidden behind the main window

This code was actually unnecessary, because I had changed the implementation to close the dialog and reopen it (while restoring geometry and position) to force it to the top If (and only if) JABS is the active application. If JABS does not have focus, then the window content is updated wherever it is without forcing it to the top. The problematic code was remaining in the code path related to opening the dialog when one does not already exist, in which case if JABS is the active application it should appear on top of the Main Window anyway and if jabs is not the active application, we shouldn't be forcing it to the top anyway.

This pull request removes a bunch of code that fiddled with the window state, and let the OS & Qt handle it however it wants and also adds some additional window flags to ensure the desired window configuration.

fixes #271

Tested on Windows and macOS

@gbeane gbeane requested review from bergsalex and keithshep January 15, 2026 03:44
@gbeane gbeane self-assigned this Jan 15, 2026
@gbeane gbeane merged commit 684dbbc into main Jan 15, 2026
2 checks passed
@gbeane gbeane deleted the bugfix/fix-training-report-close-button-on-windows branch January 15, 2026 14:27
Comment on lines 790 to 794
self._training_report_dialog = TrainingReportDialog(
self._training_report_markdown,
title=f"Training Report: {self._controls.current_behavior}",
parent=self,
parent=None,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why parent is changed to None

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self is the CentralWidget, so probably not really an appropriate parent. It should probably be either the Main Window or None, and Windows won't let you minimize it into the task bar like a normal window if it's a child of some other widget (even the Main Window). When parent is None, on Windows it will have its own entry in the task bar, which I felt made it easier to keep track of.

I could be convinced to make it a child of the Main Window in a future release though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Training Report Window Unable to Close

4 participants